Refactor WordPressSite to be a struct#25387
Conversation
Generated by 🚫 Danger |
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
3 times, most recently
from
5c8fe6c to
c4d1470
Compare
|
Version |
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
from
c4d1470 to
b1b1500
Compare
|
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
from
b1b1500 to
9834878
Compare
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 32500 | |
| Version | PR #25387 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | d77081f | |
| Installation URL | 1b7a83j6etdv0 |
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 32500 | |
| Version | PR #25387 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | d77081f | |
| Installation URL | 05povcboh8qvo |
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
2 times, most recently
from
dbc6313 to
5637585
Compare
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
from
5637585 to
31a8555
Compare
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
from
31a8555 to
46fcc0b
Compare
crazytonyli
force-pushed
the
task/refactor-site-enum
branch
from
46fcc0b to
4d33c9b
Compare
| } | ||
|
|
||
| public init(blog: Blog) throws { | ||
| extension WordPressSite: Hashable { |
There was a problem hiding this comment.
The == conformance seems like it wouldn't be correct if the flavour or URL of a site changed (which can happen if a site is prompted from WP.com simple to Atomic, for instance)
There was a problem hiding this comment.
The main reason for this conformance is that WordPressSite is used as a dictionary key in a few places, which probably is not a good idea. How about I look into refactoring those usages later and then remove this Hashable conformance?
There was a problem hiding this comment.
Works for me – let's just be sure not to lose track of it! 😅
| return siteURL | ||
| // Check for WordPress.com account first. This means Atomic sites | ||
| // (which have both an account and application password credentials) | ||
| // resolve to `.dotCom`. |
There was a problem hiding this comment.
This is fine for now, but it'll need to change down the road – we want atomic sites to talk directly to the site instead of going through WP.com.
The main area this matters is the editor, where the WP.com proxy actually breaks certain endpoints we need. I think it's something we'll address in the editor loading code though.
There was a problem hiding this comment.
Good catch. That's a behavior change, which should not happen. The related code is in the WordPressClient.init(site: WordPressSite), which I have updated in the latest commit.
The flavor property (which is not a great name...) here indicates how the site is presented in the app: atomic site can be displayed as a self-hsoted site or part of WP.com account, depending how it's signed in.
The flavor describes how a site is presented in the app, while the new transport property decides which endpoint and credentials are used for wp/v2 access, preferring direct site access whenever application password credentials are available. This restores the routing behaviour for sites that have both a WordPress.com account and an application password, such as Atomic sites. Also rename ApiFlavor to Flavor since the type no longer dictates API access.
4 participants
Description
I have mixed another change in this PR, which is integration Automattic/wordpress-rs#1239 and making the WpService instance support WP.com sites. That means, you can now browse custom post types by signing in with WP.com accounts and choosing atomic sites.
Fixes https://linear.app/a8c/issue/CMM-1958